-
Notifications
You must be signed in to change notification settings - Fork 106
chore(PythonQt): Migrate to Python-3 Unicode API; update generator/wrappers; add opt-in PyString shim; enforce Python ≥ 3 #308
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore(PythonQt): Migrate to Python-3 Unicode API; update generator/wrappers; add opt-in PyString shim; enforce Python ≥ 3 #308
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
|
My personal preference would be your second alternative. @usiems - what do you think? |
|
Option 2 is good for me. We could also go with option 3, since we might call the next version 4.0 (mostly because we dropped Python 2 support), and this would justify some incompatibilities. But option 2 is certainly safer. The generated wrappers only seem to use PyString_FromString, so, if we only care about existing generated wrapper code, it would be sufficient to keep just this one define. By the way, what about the PyInt_ defines? Shouldn't they be replaced, too? |
|
d2506eb to
fcb1caf
Compare
fcb1caf to
ed2750d
Compare
Reported error1 is unrelated to the proposed changes: Footnotes |
PyUnicode_*| @@ -50,7 +50,7 @@ PythonQtShell_QAbstractAnimation::~PythonQtShell_QAbstractAnimation() { | |||
| void PythonQtShell_QAbstractAnimation::childEvent(QChildEvent* arg__1) | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we should remove these older versions of generated code now, instead of adapting them to these changes each time.
Qt versions before 5.9 are not tested at all, and we do not test the checked-in generated code anyway, so we cannot guarantee that it will still work with the current version of PythonQt. Had also a discussion with @usiems about this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would tend to remove all checked-in versions except perhaps the 5.15 one. 5.15 can serve as an example of what kind of code is generated, but I would recommend to anyone using that Qt version to re-run the generator even so.
| #define PY3K | ||
| // Helper defines to facilitate porting | ||
| #define PyString_FromString PyUnicode_FromString |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe add a deprecation warning here? Or would this over-flood the log with warnings?
| @@ -50,7 +50,7 @@ PythonQtShell_QAbstractAnimation::~PythonQtShell_QAbstractAnimation() { | |||
| void PythonQtShell_QAbstractAnimation::childEvent(QChildEvent* arg__1) | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would tend to remove all checked-in versions except perhaps the 5.15 one. 5.15 can serve as an example of what kind of code is generated, but I would recommend to anyone using that Qt version to re-run the generator even so.
…ring` Make Unicode creation explicit instead of relying on the compatibility macros in `PythonQtPythonInclude.h`. No functional change intended.
This removes dependence on the `PyString_*` shim and documents the Python 3 assumption clearly. No functional change intended.
Port remaining conversions to the Unicode API and drop reliance on the macro alias in `PythonQtPythonInclude.h`. No functional change intended.
…rmat` Format string creation now uses the Unicode API directly. No functional change intended.
Update assertions and type checks to the Unicode API. No functional change intended.
…tring` Make Unicode creation explicit instead of relying on the compatibility macros in `PythonQtPythonInclude.h`. No functional change intended.
…_FromString` Make Unicode creation explicit instead of relying on the compatibility macros in `PythonQtPythonInclude.h`. No functional change intended.
ed2750d to
e793d1a
Compare
This PR completes the migration to Python 3 string APIs across PythonQt, updates the code generator and generated wrappers, and introduces an opt-in compatibility shim for legacy wrappers.
It removes reliance on
PyString_*aliases inPythonQtPythonInclude.hfor in-tree sources and makes Unicode usage explicit. The build now errors if Python 2.x is detected.Summary:
Replace
PyString_*with Python-3 equivalents:PyString_FromString→PyUnicode_FromStringPyString_AS_STRING→PyUnicode_AsUTF8PyString_AsString→PyUnicode_AsUTF8PyString_FromFormat→PyUnicode_FromFormatPyString_Check→PyUnicode_CheckUpdate the generator to emit
PyUnicode_*.Update generated_cpp_* wrappers to use
PyUnicode_*.Add
PYTHONQT_USE_PYSTRING_SHIM(default OFF) to aliasPyString_FromString→PyUnicode_FromStringfor older, out-of-tree wrappers.Enforce Python 3 at build time (hard error on Python 2.x).